-
-
Notifications
You must be signed in to change notification settings - Fork 453
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make SomeModel._default_manager return a BaseManager[SomeModel] instead of BaseManager[Model] #817
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Great feature, please, see my comments
django-stubs/utils/functional.pyi
Outdated
class classproperty(Generic[_Get]): | ||
fget: Optional[Callable[[Type[_Self]], _Get]] = ... | ||
def __init__(self, method: Optional[Callable[[Type[_Self]], _Get]] = ...) -> None: ... | ||
def __get__(self, instance: Any, cls: Type[_Self] = ...) -> _Get: ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I see __get__
is usually an overload
of two functions.
You can take a look here: https://github.com/python/typeshed/
@@ -48,7 +48,7 @@ | |||
class Base(Generic[_T]): | |||
def __init__(self, model_cls: Type[_T]): | |||
self.model_cls = model_cls | |||
reveal_type(self.model_cls._default_manager) # N: Revealed type is "django.db.models.manager.BaseManager[django.db.models.base.Model]" | |||
reveal_type(self.model_cls._default_manager) # N: Revealed type is "django.db.models.manager.BaseManager[_T`1]" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's also test ._base_manager
of some Model
, so we can ensure that _T
is correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm super unfamiliar with these types of tests. I hope the new test makes sense!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
I'm using pylance and found this does not work. user: Users = Users._default_manager.get(pk=user_id) shows error:
any ideas? |
This changes the type annotations for _default_manager (and _base_manager) such that the returned manager's model is the specific model the manager belongs to instead of django.db.models.base.Model